Repository navigation
doc: PWA + push notifications design for #1254 (epic #1184) - #3314
Conversation
…ts, quiet hours, Web Push branched on #1182, deep links, anti-fatigue, /sw.js disposition)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c59150591b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **Branch A — tailnet-only (today's reality).** The aggregator binds | ||
| `127.0.0.1:4870` + the tailnet name; there is no public route | ||
| (#1182 triage, verified at `f00dde689`). Subscriptions are created | ||
| from the tailnet origin while the phone is on the tailnet, pushes |
There was a problem hiding this comment.
On the current deployment, branch A cannot create the subscriptions described here: pm-stack/aggregator/README.md:38 identifies the canonical origin as http://amd-6.raptor-pumpkinseed.ts.net:4870, and src/server.ts starts plain HTTP listeners. Being connected through Tailscale does not make that remote HTTP origin a browser secure context. The Secure Contexts specification requires a secure context to register service workers, and the Push API likewise restricts PushManager to secure contexts. Add a trusted HTTPS endpoint to branch A's prerequisites and implementation slices; otherwise the recommended path cannot deliver push notifications on Drew's phone, regardless of the outbound-only sending architecture.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 76e8983: branch A now carries the trusted-HTTPS prerequisite — the canonical tailnet origin is plain HTTP (README:38), which is not a secure context, so no SW/PushManager subscription is possible on it. The design now starts branch A by serving the aggregator via Tailscale Serve HTTPS (that origin becomes PM_AGG_PUBLIC_ORIGIN), and slice (a) includes the Serve mapping + docs.
|
|
||
| | Class | Source | Trigger | Recommended default | | ||
| |---|---|---|---| | ||
| | `needs-drew` | `/api/attention.json`; the `needs-drew` tier is derived in `src/derive.ts:308-324` | an item *enters* the needs-drew tier | **on** | |
There was a problem hiding this comment.
When a decision, task, or gap appears in the dashboard's needs-drew queue without a matching session, polling /api/attention.json will never detect it. pm-stack/aggregator/src/server.ts:780-789 returns only view.all plus needs-orchestrator watchdog entries, while src/derive.ts:435-463 keeps these non-session items separately in view.drewExtra; src/render.ts:2402-2410 explicitly includes them in the displayed queue. Consequently, the specified source misses both entry notifications and blocked-age escalation for these actionable items. Have the design consume the complete shared attention view, extend the API, or incorporate the orchestration tier source before defining it as the engine's input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 76e8983: the engine source is now specified as the complete in-process attention view (view.all + view.drewExtra, derive.ts:379,463 — the non-session decisions/tasks/gaps render.ts:2402 shows), not the flat /api/attention.json array (server.ts:780-789, all+watchdog only). Slice (c) also extends /api/attention.json with the drewExtra entries so external consumers see the same queue.
…for branch A; engine source is the full attention view incl. drewExtra
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Design deliverable for #1254, per queue task 4957 (triage task 2514, CONFIRMED greenfield at a1aa07d).
The note (
doc/2026-11-01T03-54-00Z_design_pwa_push_notifications.md) answers every #1254 design question:Implementation issues are filed separately and attached as sub-issues of epic #1184, linked from #1254. Design only — no code.
Refs #1254, #1184, #1182.
Need help on this PR? Tag
@codesmith-botwith what you need.